Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix i18n _get_default_locale_path #204

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dangillet
Copy link

@dangillet dangillet commented Sep 22, 2024

Fixes #105

Changes proposed in this pull request:

  • Use importlib.resources available since Python 3.9 to find the location of the locale folder containing the translations.

@hugovk hugovk added the changelog: Fixed For any bug fixes label Sep 22, 2024
Fixes python-humanize#105

Use importlib.resources available since Python 3.9 to find the location
of the locale folder containing the translations.
@dangillet
Copy link
Author

I tried to use __package__ initially, but the docs suggest to use __spec__.parent instead. https://docs.python.org/3/reference/import.html#package__

The tests cover when __spec__ is None or even if it's missing. As __spec__.parent is read-only , I cannot make a test which sets a different value for it.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

Let's do some tuning to speed up with lazy imports, which can be important for CLIs where you want a quick response time.

Using https://github.com/nschloe/tuna we can visualise the import times:

python -m pip install tuna
python -c "import humanize" && python -X importtime -c "import humanize" 2> import.log && tuna import.log

Here's main:

image

Here's this PR:

image

With the suggestions:

image

return None

with importlib.resources.as_file(importlib.resources.files(package)) as pkg:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
with importlib.resources.as_file(importlib.resources.files(package)) as pkg:
import importlib.resources
with importlib.resources.as_file(importlib.resources.files(package)) as pkg:

Comment on lines +6 to 9
import importlib.resources
import os
import pathlib
from threading import local
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import importlib.resources
import os
import pathlib
from threading import local
from threading import local
from typing import TYPE_CHECKING
if TYPE_CHECKING:
import os
import pathlib

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: Fixed For any bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some tests fails with pytest-randomly
2 participants